fix(ci): nolint should be written without leading space#2059
fix(ci): nolint should be written without leading space#2059tac0turtle merged 1 commit intoevstack:mainfrom
Conversation
WalkthroughThe pull request removes several lint suppression directives ( Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
RollkitBot
requested review from
a team,
Manav-Aggarwal,
tzdybal and
yarikbratashchuk
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
rpc/json/helpers_test.go (1)
52-52: Consistent nolint formatting applied.
The extra space before thenolintdirective has been removed (i.e. now//nolint:errcheck,gosec), which aligns with the updated coding standards and the PR objective.cmd/rollkit/commands/intercept.go (2)
80-80: Remove extra whitespace in nolint directive.
The comment has been updated from// nolint:errcheck,gosecto//nolint:errcheck,gosec, ensuring adherence to the standard formatting.
98-98: Verify consistent nolint style across the file.
Please double-check that all instances of nolint comments in this file follow the//nolint:errcheck,gosecstyle uniformly.test/e2e/sut_helper.go (2)
242-242: Consistent nolint formatting for file read.
The updated comment (//nolint: gosec // used by tests only) now has the extra space removed, aligning with the new convention.
248-248: Consistent nolint formatting for file creation.
Similarly, the change at this location removes the leading space beforenolint: gosec, ensuring consistency in the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/rollkit/commands/intercept.go(1 hunks)da/da_test.go(1 hunks)rpc/json/helpers_test.go(1 hunks)test/e2e/sut_helper.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test / Run Unit Tests
- GitHub Check: Summary
🔇 Additional comments (1)
da/da_test.go (1)
63-63: Standardize nolint directive in TestMain.
The removal of the leading space in the comment (now//nolint:errcheck,gosec) is in line with the updated linting rules and the PR objective.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2059 +/- ##
==========================================
- Coverage 42.06% 42.00% -0.06%
==========================================
Files 80 80
Lines 12950 12947 -3
==========================================
- Hits 5448 5439 -9
- Misses 6777 6783 +6
Partials 725 725 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tac0turtle
left a comment
tac0turtle
left a comment
There was a problem hiding this comment.
could you look into removing the nolints? is it possible?
Removing the |
this and potentially others? it would be only removing and seeing if lint passes instead of trying to fix the issue |
|
I tried, and there are a few cases(which may be added by mistake) that we can remove the |
amazing, thank you for looking into this. want to push the ones we can remove |
Okay, I will do this later |
suiyuan1314
force-pushed
the
fix/nolint
branch
2 times, most recently
from
5957c9f to
7f94ca0
Compare
2 participants
Summary by CodeRabbit